lastgenre: Blacklist wrong last.fm genres#5744
lastgenre: Blacklist wrong last.fm genres#5744JOJ0 wants to merge 3 commits intolastgenre_split_monolithfrom
Conversation
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
256a8a1 to
c8199cb
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## lastgenre_split_monolith #5744 +/- ##
============================================================
+ Coverage 69.57% 69.59% +0.01%
============================================================
Files 142 142
Lines 18508 18598 +90
Branches 3026 3054 +28
============================================================
+ Hits 12877 12943 +66
- Misses 4995 5014 +19
- Partials 636 641 +5
🚀 New features to boost your workflow:
|
9cd4947 to
a0e6054
Compare
a3daa01 to
4868326
Compare
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
5b24d49 to
13a01b1
Compare
|
Setting this back to draft. I'd like to fix this bug first: #5930 |
58c1299 to
9e5f168
Compare
9ff6150 to
0f131b5
Compare
723345e to
81c3219
Compare
b1e9732 to
22de6ee
Compare
998f9a6 to
751d5d1
Compare
81c3219 to
f07cb30
Compare
42b0d45 to
e49d7a3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
beetsplug/lastgenre/init.py:474
- grug see log label suffix only say "whitelist" or "any". but now blacklist can filter too, so label "any" lie when blacklist active. grug want suffix reflect blacklist (ex: "any+blacklist" or similar) so debug logs tell truth.
suffix = "whitelist" if self.whitelist else "any"
label = f"{stage_label}, {suffix}"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
beetsplug/lastgenre/init.py:483
- grug see log label say
whitelistvsanybased only onself.whitelist. but now blacklist can filter too. when whitelist off but blacklist on, label sayanyeven though not any. please include blacklist state in label (ex:blacklist, orwhitelist+blacklist) so logs tell truth.
suffix = "whitelist" if self.whitelist else "any"
label = f"{stage_label}, {suffix}"
if keep_genres:
label = f"keep + {label}"
return self._format_genres(resolved_genres), label
beetsplug/lastgenre/init.py:322
- grug see _resolve_genres docstring still talk only about whitelist filtering. now blacklist also filter and can skip c14n parent walk. please update docstring bullets so match new behavior (whitelist+blacklist).
"""Canonicalize, sort and filter a list of genres.
- Returns an empty list if the input tags list is empty.
- If canonicalization is enabled, it extends the list by incorporating
parent genres from the canonicalization tree. When a whitelist is set,
only parent tags that pass the whitelist filter are included;
otherwise, it adds the oldest ancestor. Adding parent tags is stopped
when the count of tags reaches the configured limit (count).
- The tags list is then deduplicated to ensure only unique genres are
retained.
- If the 'prefer_specific' configuration is enabled, the list is sorted
by the specificity (depth in the canonicalization tree) of the genres.
- Finally applies whitelist filtering to ensure that only valid
genres are kept. (This may result in no genres at all being retained).
- Returns the filtered list of genres, limited to the configured count.
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
beetsplug/lastgenre/init.py:486
- grug see log label pick suffix "any" when whitelist off. but now blacklist can be on, so it not really "any". log say wrong thing, debug harder. suggest include blacklist state in suffix (ex: "blacklist" or "any+blacklist").
if resolved_genres:
suffix = "whitelist" if self.whitelist else "any"
label = f"{stage_label}, {suffix}"
if keep_genres:
| Tries regex compilation first, falls back to literal string matching. | ||
| That way users can use regexes for flexible matching but also simple | ||
| strings without worrying about regex syntax. All patterns are | ||
| case-insensitive. |
| if filtered_genre != genre: | ||
| log_filtered = set(genre) - set(filtered_genre) | ||
| extra_debug(self._log, "blacklisted: {}", log_filtered) | ||
| genre = filtered_genre |
- Test file format (valid and error cases) - Test regex pattern matching (_is_blacklisted) - Test _resolve_genres: blacklisted genres filtered - Test _resolve_genres: c14n ancestry walk blocked for blacklisted tags
- Prevents wrong last.fm genres based on a per artist list of forbidden regex patterns. Blacklisting happens in two places: Right after fetching the last.fm genre and in _resolve_genres. - Includes docs for the new feature.
fix fallback yptes
Description
Adds a global and artist-specific genre blacklist to lastgenre. Blacklist entries can use regex patterns or literal genre names and are configurable per artist or globally (*). Example:
This avoids incorrect genre assignments for artists with overlapping names (e.g., “Fracture” the DnB producer vs a Metal band)..
Why invent a file format?
YAML and INI formats were tested but proved cumbersome for regex patterns. A simple custom format is user-friendly and allows all regex patterns without special escaping or formatting.
Further notes
My personal roadmap for the
lastgenreplugin: #5915Feature idea originated from: #5721 (comment).
To Do